Add --resolve-conflict flag to transport version generate task #134421

rjernst · 2025-09-10T04:27:31Z

When merge conflicts arise with transport versions we want to be able to regenerate ids for all the branches currently defined in the newly added transport version definition. This commit adds a --resolve-conflict flag to the generate task which figures out the appropriate branches automatically from the outstanding definition. With this the command line to regenerate on a merge conflict is as simple as:

./gradlew generateTransportVersion --resolve-conflict

When merge conflicts arise with transport versions we want to be able to regenerate ids for all the branches currently defined in the newly added transport version definition. This commit adds an `--update` flag to the generate task which figures out the appropriate branches automatically from the outstanding definition. With this the command line to regenerate on a merge conflict is as simple as: `./gradlew generateTransportVersion --update`

elasticsearchmachine · 2025-09-10T04:27:56Z

Pinging @elastic/es-core-infra (Team:Core/Infra)

prdoyle · 2025-09-10T14:27:22Z

Could this "just work" without specifying --update? The mental model where I just run ./gradlew generateTransportVersion to fix stuff will be easier for users to remember.

If the answer is "no", then let's rename the flag to something more explicit. Since generateTransportVersion is designed to be idempotent, people will think that generateTransportVersion already "updates" the TV, and so the flag --update won't mean much. Perhaps --resolve-conflict?

mark-vieira · 2025-09-10T15:25:34Z

If the answer is "no"

The answer is indeed no as we cant distinguish between --backport-branches missing because we actually want to remove those backports or we've just omitted the option. We don't want people to have to pass that.

Perhaps --resolve-conflict?

Yeah, but that's not really what it's doing. This is what you run after you resolve the conflict by doing a merge.

Really what we're trying to communicate with this flag is it's "non-destructive". It's not going to remove any existing transport ids, just "update" or "refresh" them to align with upstream changes.

mark-vieira · 2025-09-10T15:28:53Z

...groovy/org/elasticsearch/gradle/internal/transport/TransportVersionGenerationFuncTest.groovy

+        referableAndReferencedTransportVersion("new_tv", "8123000")
+        file("myserver/src/main/resources/transport/latest/9.2.csv").text =
+            """
+            <<<<<<< HEAD


The intention is that folks do a merge (address conflicts) and then run this task right? So we should never be in a position where we have conflicts in files? I mean either way, the content of the file is sort of irrelevant as we're just going to override it so the test is still valid as-is. Just want to understand the intent here.

Yes that's the intention. You're right that the contents don't matter. I just made the test mimic the scenario as much as possible, and this is what it would like during a merge with a conflict.

I would think the dev shouldn't have to resolve the conflicts in the generated files at all. I assume this is the intention still? So if that's the only conflict they can just run this gradle task again.

I would think the dev shouldn't have to resolve the conflicts in the generated files at all. I assume this is the intention still?

I don't think we can avoid that. They are going to have to resolve those conflicts just like any other. The thing is that it doesn't really matter how they are resolved since we'll just fix it.

mark-vieira · 2025-09-10T15:32:11Z

.../main/java/org/elasticsearch/gradle/internal/transport/TransportVersionResourcesService.java

        Files.writeString(path, upperBound.definitionName() + "," + upperBound.definitionId().complete() + "\n", StandardCharsets.UTF_8);
+
+        if (stageInGit) {
+            gitCommand("add", path.toString());


Is this really necessary? I mean we don't do this for any other kind of generation tasks. Can we not just rely on folks to do this themselves just as with any other local changes that need to be committed/pushed?

It's not necessary, but I think it's good. I don't think devs should think about the files that need to be added, when there is a conflict, they just run this command, and these files will no longer show as unresolved. Otherwise they would have to add themselves (or run git commit -a, but not everyone does that with merge conflicts).

I mean we don't do this for any other kind of generation tasks

We could consider doing this for other files...

I worry if we don't add the files it would actually be more confusing since they are absolutely necessary to run ES so I'm +1 on this.

I don't have super strong feelings on this but I'm generally -1 on things that muck with folks' local git state. There's just so many unique workflows people use and we just don't know when we might be stepping on those.

I suppose the question is "how magic is the tool/transport versions". We don't auto-stage spotlessApply because it's touching code we wrote. No magic. But we do auto-stage it when CI does spotlessApply for us.

If transport versions are managed by the tool almost entirely, then, yeah, staging is nice and useful.

As far as commit -a - whenever I get a merge conflict I manually git add each resolved file one by one on the command line. But I'm old now. I have no idea what most folks do.

I'd be really creeped out if it auto-staged files it isn't managing. But that's not this.

If transport versions are managed by the tool almost entirely, then, yeah, staging is nice and useful.

That's a fair distinction. In which case to my previous comment, I would then expect it to auto stage for any changes it makes to files it manages, not just when passing --resolve-conflict.

I was just thinking maybe it should auto-stage for anything involving the internal transport version files because we really don't expect or want users to have to consider these at all.

I don't think we expect folks to understand all those distinctions. Basically we don't expect folks to ever manually touch any of these files. If that's the case, then I agree with Nik that the task should just "own" these files and handle staging them in all scenarios if that's what we're gonna do.

jdconrad · 2025-09-10T16:56:43Z

...java/org/elasticsearch/gradle/internal/transport/GenerateTransportVersionDefinitionTask.java


+    @Input
+    @Optional
+    @Option(option = "update", description = "Update the transport version currently being added to upstream")


I like @mark-vieira's suggestion of refresh better than update. Update implies we are working on something that already exists which the version being worked on doesn't.

Per a suggestion from @nik9000 I've changed this to --resolve-conflict since that is what a developers is trying to do, resolve a merge conflict

Oh hey. Neat.

Per a suggestion from @nik9000 I've changed this to --resolve-conflict since that is what a developers is trying to do, resolve a merge conflict

I'm still not a fan, mainly because just running this task doesn't actually resolve the conflict. You still have to merge upstream changes and then run this task. I'll defer my objections though if that makes intuitive sense to most folks. I might be too close to this work to provide an objective opinion here.

mark-vieira · 2025-09-10T18:52:27Z

It's worth noting that rather than having a single task and exposing this behavior as a cli option, we could just register another task with a better name that's preconfigured to do the right thing. For example just register a fixTransportVersions task and make this "resolve conflict" or "update" or whatever property be internal and set it by the plugin.

prdoyle · 2025-09-10T19:27:34Z

we cant distinguish between --backport-branches missing because we actually want to remove those backports or we've just omitted the option.

Does that indicate that we've neglected to collect certain requirements from the user? Why can't we tell this?

prdoyle · 2025-09-10T19:28:33Z

+1 on a separate task. Seems more Gradley.

mark-vieira · 2025-09-10T19:43:14Z

we cant distinguish between --backport-branches missing because we actually want to remove those backports or we've just omitted the option.

Does that indicate that we've neglected to collect certain requirements from the user? Why can't we tell this?

Yes, because running the task with an empty set is perfectly fine, and actually the way most people will run it. Also, it's just super easy to forget.

rjernst · 2025-09-11T14:17:03Z

we could just register another task with a better name

While that might work, the implementation will be annoying (having to duplicate much of the logic from the generate task). And I think it's simpler for devs to remember one task name for transport versions with a flag than two different tasks, but that's just a gut feeling which is easier to remember.

rjernst · 2025-09-11T14:19:58Z

Yeah, but that's not really what it's doing. This is what you run after you resolve the conflict by doing a merge.

I don't think that's true. It's what you run after you run git merge when you have conflicts, and this is how you resolve conflicts within transport state files.

mark-vieira · 2025-09-11T17:13:57Z

While that might work, the implementation will be annoying (having to duplicate much of the logic from the generate task).

My suggestion was just to use the same task type, just configure it differently so you wouldn't have to actually duplicate implementation logic.

And I think it's simpler for devs to remember one task name for transport versions with a flag than two different tasks, but that's just a gut feeling which is easier to remember.

Yeah, that might be true. Just figured I'd make it known that's an option.

…ic#134421) When merge conflicts arise with transport versions we want to be able to regenerate ids for all the branches currently defined in the newly added transport version definition. This commit adds a --resolve-conflict flag to the generate task which figures out the appropriate branches automatically from the outstanding definition. With this the command line to regenerate on a merge conflict is as simple as: ./gradlew generateTransportVersion --resolve-conflict

elasticsearchmachine · 2025-09-12T17:27:34Z

💚 Backport successful

Status	Branch	Result
✅	8.19
✅	9.1
✅	8.18
✅	9.0

…ic#134421) When merge conflicts arise with transport versions we want to be able to regenerate ids for all the branches currently defined in the newly added transport version definition. This commit adds a --resolve-conflict flag to the generate task which figures out the appropriate branches automatically from the outstanding definition. With this the command line to regenerate on a merge conflict is as simple as: ./gradlew generateTransportVersion --resolve-conflict

…) (#134662) When merge conflicts arise with transport versions we want to be able to regenerate ids for all the branches currently defined in the newly added transport version definition. This commit adds a --resolve-conflict flag to the generate task which figures out the appropriate branches automatically from the outstanding definition. With this the command line to regenerate on a merge conflict is as simple as: ./gradlew generateTransportVersion --resolve-conflict

…) (#134661) When merge conflicts arise with transport versions we want to be able to regenerate ids for all the branches currently defined in the newly added transport version definition. This commit adds a --resolve-conflict flag to the generate task which figures out the appropriate branches automatically from the outstanding definition. With this the command line to regenerate on a merge conflict is as simple as: ./gradlew generateTransportVersion --resolve-conflict

…) (#134660) When merge conflicts arise with transport versions we want to be able to regenerate ids for all the branches currently defined in the newly added transport version definition. This commit adds a --resolve-conflict flag to the generate task which figures out the appropriate branches automatically from the outstanding definition. With this the command line to regenerate on a merge conflict is as simple as: ./gradlew generateTransportVersion --resolve-conflict

…) (#134663) When merge conflicts arise with transport versions we want to be able to regenerate ids for all the branches currently defined in the newly added transport version definition. This commit adds a --resolve-conflict flag to the generate task which figures out the appropriate branches automatically from the outstanding definition. With this the command line to regenerate on a merge conflict is as simple as: ./gradlew generateTransportVersion --resolve-conflict

…ic#134421) When merge conflicts arise with transport versions we want to be able to regenerate ids for all the branches currently defined in the newly added transport version definition. This commit adds a --resolve-conflict flag to the generate task which figures out the appropriate branches automatically from the outstanding definition. With this the command line to regenerate on a merge conflict is as simple as: ./gradlew generateTransportVersion --resolve-conflict

…ic#134421) (elastic#134660) When merge conflicts arise with transport versions we want to be able to regenerate ids for all the branches currently defined in the newly added transport version definition. This commit adds a --resolve-conflict flag to the generate task which figures out the appropriate branches automatically from the outstanding definition. With this the command line to regenerate on a merge conflict is as simple as: ./gradlew generateTransportVersion --resolve-conflict

rjernst requested a review from a team as a code owner September 10, 2025 04:27

rjernst added >non-issue :Core/Infra/Core Core issues without another label auto-backport Automatically create backport pull requests when merged v8.19.5 v9.1.5 v8.18.8 v9.0.8 labels Sep 10, 2025

rjernst requested review from jdconrad and mark-vieira September 10, 2025 04:27

elasticsearchmachine added v9.2.0 Team:Core/Infra Meta label for core/infra team labels Sep 10, 2025

mark-vieira approved these changes Sep 10, 2025

View reviewed changes

jdconrad approved these changes Sep 10, 2025

View reviewed changes

rjernst added 2 commits September 10, 2025 11:28

update flag name

1f80cee

iter

e005705

rjernst changed the title ~~Add --update flag to transport version generate task~~ Add --resolve-conflict flag to transport version generate task Sep 10, 2025

[CI] Auto commit changes from spotless

2dcec3d

rjernst added 2 commits September 11, 2025 09:01

fix flag in tests

1d264ff

Merge branch 'main' into transport/generate_update_flag

99990c6

rjernst merged commit 47acb9a into elastic:main Sep 12, 2025
35 checks passed

rjernst deleted the transport/generate_update_flag branch September 12, 2025 17:25

rjernst mentioned this pull request Sep 12, 2025

[8.19] Add --resolve-conflict flag to transport version generate task (#134421) #134660

Merged

This was referenced Sep 12, 2025

[9.1] Add --resolve-conflict flag to transport version generate task (#134421) #134661

Merged

[8.18] Add --resolve-conflict flag to transport version generate task (#134421) #134662

Merged

rjernst mentioned this pull request Sep 12, 2025

[9.0] Add --resolve-conflict flag to transport version generate task (#134421) #134663

Merged

Add --resolve-conflict flag to transport version generate task #134421

Add --resolve-conflict flag to transport version generate task #134421

Uh oh!

Conversation

rjernst commented Sep 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

elasticsearchmachine commented Sep 10, 2025

Uh oh!

prdoyle commented Sep 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mark-vieira commented Sep 10, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mark-vieira Sep 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jdconrad Sep 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rjernst Sep 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mark-vieira Sep 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mark-vieira commented Sep 10, 2025

Uh oh!

prdoyle commented Sep 10, 2025

Uh oh!

prdoyle commented Sep 10, 2025

Uh oh!

mark-vieira commented Sep 10, 2025

Uh oh!

rjernst commented Sep 11, 2025

Uh oh!

rjernst commented Sep 11, 2025

Uh oh!

mark-vieira commented Sep 11, 2025

Uh oh!

Uh oh!

elasticsearchmachine commented Sep 12, 2025

💚 Backport successful

Uh oh!

Reviewers

Assignees

Labels

Projects

rjernst commented Sep 10, 2025 •

edited

Loading

prdoyle commented Sep 10, 2025 •

edited

Loading

mark-vieira Sep 10, 2025 •

edited

Loading

jdconrad Sep 10, 2025 •

edited

Loading

rjernst Sep 10, 2025 •

edited

Loading

mark-vieira Sep 10, 2025 •

edited

Loading